add document defining an OpenTelemetry Collector #4313
Conversation
Related to open-telemetry#4309 Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
Co-authored-by: Reiley Yang <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
Co-authored-by: Reiley Yang <[email protected]>
specification/collector/README.md
Outdated
| For a library to be considered an OpenTelemetry Collector component, it _MUST_ | ||
| implement the [Component interface](https://github.com/open-telemetry/opentelemetry-collector/blob/main/component/component.go) |
There was a problem hiding this comment.
The Collector also accepts confmap.Providers and confmap.Converters, which do not accept this interface. Do we consider those out of scope?
There was a problem hiding this comment.
I think they do need to be considered in scope. Interoperability of those components is important.
There was a problem hiding this comment.
The Collector also accepts confmap.Providers and confmap.Converters, which do not accept this interface. Do we consider those out of scope?
I wonder if including them would allow us to avoid having to include a definition for a config file, wdyt?
There was a problem hiding this comment.
Providers handle configuration abstractly so they help remove the need for how configuration should be represented, but they don't solve the schema part (which I don't feel like we need to solve tbh)
| The goal of this document is for users to be able to easily switch between | ||
| OpenTelemetry Collector Distros while also ensuring that components produced by | ||
| the OpenTelemetry Collector SIG are able to work with any vendor who claims | ||
| support for an OpenTelemetry Collector. |
There was a problem hiding this comment.
I'm not sure I understand this goal. If a vendor produces a collector distribution that has a subset of available components because those are the components relevant to their service offerings and that they're willing to support, where do any other components (whether hosted in an OTel repo or not) fit into that picture? Do we mean that a distribution must offer end users the ability to modify its source and create their own build? We should be explicit about that if that is the case.
Given that the licensing of the collector's source code does not require that distribution of derivative works happen in source form I'm not sure that we have much ability here to enforce such a requirement. We can certainly try to use the "OpenTelemetry" mark as a cudgel, but I'm not sure it'll be as effective as may be desirable since the terms "collector" and "distribution" are very broad. It could perhaps be argued that "OpenTelemetry Collector" is a protectable mark and maybe even that "Collector" has acquired secondary meaning in this limited scope, but protecting such a mark against genericization is going to be a Sisyphean task.
There was a problem hiding this comment.
I see this definition as separation from the term Distribution defined below. A Distribution is a specific compiled OpenTelemetry Collector with a specific set of OpenTelemetry Collector Components that the maintainer (the user in this case) decided to add. It is a OpenTelemetry Collector bc the maintainer was able to bring their chosen OpenTelemetry Collector components to it.
Something is not an OpenTelemetry Collector if it cannot support OpenTelemetry Collector Components. Maybe the word additional below is unnecessary and could be removed?
There was a problem hiding this comment.
Given that the licensing of the collector's source code does not require that distribution of derivative works happen in source form I'm not sure that we have much ability here to enforce such a requirement
We potentially have leverage over:
- Trademark usage if "OpenTelemetry Collector" becomes a trademark
- What we list on our registry and website and what we promote
- What wording can be used in 'official' OTel events
I think we have enough leverage here to make this worth it
There was a problem hiding this comment.
I'm not sure that having a registered "OpenTelemetry Collector" mark is sufficient here as nominative fair use would allow anyone preparing a distribution (in the colloquial sense, not Distribution however we seek to define it) to identify it as such. The Linux Foundation trademark usage guidelines also call out specifically this sort of usage as acceptable for indicating products are related to or based on the project that produces the product bearing their marks.
Obviously the project can control what it puts on its website and what marketing collateral is used in conjunction with events operated by LF/CNCF, but that doesn't seem like effective leverage over an actor who has no need or interest in such things.
There was a problem hiding this comment.
I would generally say if someone isn't interested in "playing nice" then it doesn't really matter what we say or what we don't say. The solution to enforceable marks is offering certification and conformance suites that are attached to actual trademarks (e.g., "OTLP Inside" or whatever). This document is guidance for the community as much as it is guidance for external parties.
There was a problem hiding this comment.
What @austinlparker said along with my thinking that its important to define this, and include this requirement, to make clear why opentelemetry.io would or wouldn't list project Y as a Collector or Distribution.
specification/collector/README.md
Outdated
| For a library to be considered an OpenTelemetry Collector component, it _MUST_ | ||
| implement the [Component interface](https://github.com/open-telemetry/opentelemetry-collector/blob/main/component/component.go) |
There was a problem hiding this comment.
I think they do need to be considered in scope. Interoperability of those components is important.
Co-authored-by: Anthony Mirabella <[email protected]>
| to: collector/README.md | ||
| ---> | ||
|
|
||
| # OpenTelemetry Collector |
There was a problem hiding this comment.
OpenTelemetry Collector is never defined. Is it a source code artifact? A binary?
There was a problem hiding this comment.
That's one of the things I was trying to get at here. Since there's no binary plugin mechanism it seems that the source would need to be available for it to be extended in the manner contemplated, but that's not clear or explicit in the current state.
There was a problem hiding this comment.
Since there's no binary plugin mechanism it seems that the source would need to be available for it to be extended in the manner contemplated, but that's not clear or explicit in the current state.
Is the lack of binary plugin mechanism something that the OpenTelemetry Collector SIG wants to solve? Are there technical blockers?
Binary and dynamic loading plugin seem to be an established pattern. For example:
-
https://docs.fluentbit.io/manual/administration/configuring-fluent-bit/yaml/plugins-section
While Fluent Bit comes with a variety of built-in plugins, it also supports loading external plugins at runtime. This feature is especially useful for loading Go or Wasm plugins that are built as shared object files (.so).
There was a problem hiding this comment.
The Fluent Bit example is not necessarily apposite as it involves a C application dynamically loading shared libraries built with the Go toolchain using CGO (which is generally prohibited in the Collector codebase).
Go does have a native plugin mechanism, though it comes with many caveats and is widely regarded as a bad idea that can't be dropped due to compatibility guarantees. Its documentation sums up its litany of restrictions in this way, which sounds a lot like a suggestion to use something like ocb:
Together, these restrictions mean that, in practice, the application and its plugins must all be built together by a single person or component of a system. In that case, it may be simpler for that person or component to generate Go source files that blank-import the desired set of plugins and then compile a static executable in the usual way.
I feel we might be heading towards the wrong direction. This is what I'm seeing:
If we can solve 1) (support binary plugins), then we don't have problem 2 and 3. Which I think is the right direction to go. From the conversation here, it looks we don't try to solve 1), then we are forced to pick a battle between 2 and 3. I suspect if this approach would work out. |
If at some point in the future we solve point 1 then we can re-evaluate this decision. However, given the current state of things, I feel like this proposal clarifies what's important to us. |
Agree, I feel it is important to clarify in the doc regarding "what's the current state" and "where we want to be", so the readers won't be confused. And we should also make sure there is room for things to be improved/fixed/evolved without breaking changes and big surprises. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
There was a problem hiding this comment.
I don't see my top questions addressed in the text. The doc starts with An OpenTelemetry Collector MUST accept .. configuration, yet it never defines what "OpenTelemetry Collector" is, so what is this requirement referring to?
The only place that comes close to a definition is the Distro section, but it has its own major problem - it introduces a copy-left requirement, even though it calls Distro a compiled instance.
|
Here is the summary of my feedback:
I support that we mark the doc as Draft and move things forward, as long as there is a clear goal. (e.g., https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/metrics we've explained the major parts that will be covered by the spec). It is unclear what was/is the goal after going through this PR and the corresponding issue #4309. |
|
+1 to going ahead as Draft/Development status (status is already mentioned as such in this PR). Specially useful to make progress and work on the refinements as follow ups (define what a collector is, define what we have today vs what we want, etc). |
Respectfully, this issue is about defining the scope of the project, which is a reserved right of the GC (per https://github.com/open-telemetry/community/blob/main/governance-charter.md). The PR has multiple approvals from GC members, and a path forward for addressing feedback in follow-ups. I would encourage us to merge this PR and then hash out remaining issues in followups. edit: Not to belabor the point, but some of the feedback that has been raised doesn't feel productive in the slightest. "It doesn't define what a Collector is"? It's clearly laid out in the first paragraph. A collector must accept a config file and it must be able to include compatible collector components. That is a definition. It is a requirement. |
If this is the intention of the PR, consider updating the PR title, current it says "add document defining an OpenTelemetry Collector". And maybe clarify "this PR doesn't try to define what OpenTelemetry Collector is". |
It does define what an OpenTelemetry Collector is, though. My point is that such a definition is about project scope. |
|
Like, I really don't have any desire to turn this into a "who does what" issue but I am finding it extremely difficult to understand the opposition to this PR. A plain reading shows that it lays out a definition and a requirement for what a Collector is, by way of what a Collector does and how it does it. It is in response to actual user questions and solves a real problem around the scope of support for the project and to give guidance to third parties about how they should integrate with OpenTelemetry. Is the problem, then, that the TC feels like this document is out of scope for the Specification? If so, that's fine, we can just publish this guidance in Community. Or is there some other reason? |
It really doesn't. This is the entirety of what it says about what an "OpenTelemetry Collector" is:
"compatible" components are similarly ill-defined, and recursively reference the "OpenTelemetry Collector" "definition":
For a document using BCP14 terminology it is woefully inadequate at describing exactly what a conformant implementation would look like or how to prove conformance. I think this is the point @yurishkuro has been making from the beginning. IMO this document does not belong in the specification in this form and would be confusing at best as a community guideline on opentelemetry.io. |
|
We discussed this at today's GC meeting and have the following recommendations:
Thus, we would suggest that this PR be closed and re-submitted as a requirements documentation/guidance to implementors in the Collector repository. |
|
As requested, i've opened a PR in the collector repo to move the document there: open-telemetry/opentelemetry-collector#12435 I don't know how the discussion will differ in that repo from the one here, but feel free to suggest improvements there. |
Changes
Adds a definition of an OpenTelemetry Collector